Does it make sense for us to put an assertion before we force the re-render that divRef is undefined?

That way, we know that the re-render definitely happened, as opposed to a scenario where something funky happens and the contents of renderA() were being rendered the whole time

yeah I think it's a good idea to keep a check there - in this case though we'll actually be checking that it's set to the right element after the first render - what we're testing here is basically that the ref is called the right way on the first render and then is not overwritten with null

Does this field ever get used? Can we remove it?

ahh indeed it does not 🤦‍♀️

Does this field ever get used? Can we remove it?

Does it make sense for us to put an assertion before we force the re-render that divRef is undefined?

That way, we know that the re-render definitely happened, as opposed to a scenario where something funky happens and the contents of renderA() were being rendered the whole time

Hmmm I don't think I understood this the first time around..

Is the idea here that after the re-render, divRef hasn't changed?

yeah, basically the tests are checking that in these situations the issue which showed up in Framework doesn't happen, in the one because we're doing a relatively simple reorder and the other because we're setting key attributes (on that one if you remove the key attr then the test should fail)

Gotcha, I think where my understanding falls apart is that there's only one place we assign this.divRef, https://github.com/ionic-team/stencil/pull/3572/files/5549f41e2500b0a7104cf6ae7c1cfa5ca821443f#diff-ec92564e32a1aaa4fbee8ec77fa6269cdfb57581ca28f2d4764c276360fa60abR1109.

Should that also be on the div returned by renderB, or am I not know what I don't know? 😆

right so that ref function on there will be called when the related DOM node is created and when it is destroyed. So when the component first renders we call the renderA method and the ref will fire w/ the new DOM node as an argument. Then later on we re-render, and basically what we're testing is that when we re-render with the children in a different order the updateChildren algorithm for detecting when child nodes in the new and old children are the same works correctly.

Basically if that algorithm is not working correctly then what will happen is that on render no. 2 we'll trigger that ref race condition. In that scenario, Stencil's vdom doesn't recognize that the output of renderA is present both in new children and old children, and so it will remove the DOM nodes associated with renderA in the old children and create new ones for the new children. If this happens in a particular order due to the order of the child nodes in the old and new children then the removal of the old nodes will happen after the addition of the new ones, causing the ref callback to be called 3 times in total

1. on the first re-render
2. when the new child is added for render no. 2
3. and then again with null as the child from render no. 1 is removed

So! Basically the test with the keys is testing out a scenario where the arrangement of the children will trigger this issue if the keys aren't present.

does that all make sense? possibly I should add some more comments to the tests describing what we're trying to test here

Yep! That all makes sense - I do like the idea of adding a few more comments here for future folks reading these, would you please add those?

will do!

@rwaskiewicz ok just pushed a slight update to add some comments explaining what's up